Skip to content

test: enable 9 already-passing Node compat tests#33556

Open
nathanwhitbot wants to merge 13 commits intodenoland:mainfrom
nathanwhitbot:fix/node-compat-iter36
Open

test: enable 9 already-passing Node compat tests#33556
nathanwhitbot wants to merge 13 commits intodenoland:mainfrom
nathanwhitbot:fix/node-compat-iter36

Conversation

@nathanwhitbot
Copy link
Copy Markdown
Contributor

Summary

Enables 9 Node.js compat tests that already pass on main without any code changes. No production code touched — just tests/node_compat/config.jsonc entries.

  • parallel/test-child-process-exec-any-shells-windows.js (Windows-only — Linux/macOS skip via common.skip)
  • parallel/test-fs-realpath-on-substed-drive.js (Windows-only — Linux/macOS skip)
  • parallel/test-http-keepalive-override.js
  • parallel/test-http-pipeline-assertionerror-finish.js
  • parallel/test-http2-max-concurrent-streams.js
  • parallel/test-http2-server-settimeout-no-callback.js
  • parallel/test-http2-socket-close.js
  • parallel/test-https-byteswritten.js
  • parallel/test-tls-startcom-wosign-whitelist.js

Test plan

  • Each test verified via cargo test --test node_compat -- <name> (all 1/1 pass)
  • Re-ran each test 3× to rule out flakiness — all stable

All 9 tests already pass against current main without any code changes;
this just adds them to config.jsonc so they're tracked. Verified each
individually via `cargo test --test node_compat -- <name>` and re-ran
each three times to confirm they're not flaky.

  parallel/test-child-process-exec-any-shells-windows.js (Linux skips
    via common.skip)
  parallel/test-fs-realpath-on-substed-drive.js (Linux skips)
  parallel/test-http-keepalive-override.js
  parallel/test-http-pipeline-assertionerror-finish.js
  parallel/test-http2-max-concurrent-streams.js
  parallel/test-http2-server-settimeout-no-callback.js
  parallel/test-http2-socket-close.js
  parallel/test-https-byteswritten.js
  parallel/test-tls-startcom-wosign-whitelist.js
Copy link
Copy Markdown
Contributor Author

@nathanwhitbot nathanwhitbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Config-only — purely additive entries to tests/node_compat/config.jsonc.
Verified each new line is alphabetically placed within its section (with the
ASCII collation the file uses, e.g. keep-alive-… before keepalive-…
because - < e, and setLocalWindowSize before settimeout-… because
L < t). All 9 entries pass that check.

Two of the additions (test-child-process-exec-any-shells-windows.js and
test-fs-realpath-on-substed-drive.js) are Windows-only and self-skip via
common.skip on non-Windows runners, so no platform gate is needed in the
config — the runner counts the skip as a pass. Worth keeping an eye on the
Windows runner specifically, since those two are the only ones exercising
real code paths there.

The other seven look low-risk to enable. The http2 batch
(max-concurrent-streams, server-settimeout-no-callback, socket-close)
is the area I'd watch most closely for after-the-fact flakiness given the
recent pattern in this suite — happy to mark/ignore if anything surfaces.
No production-code changes mean review surface here is small. LGTM modulo
post-merge CI watch.

- test-child-process-exec-any-shells-windows.js: drop (Windows-specific, fails on Windows aarch64).
- test-fs-realpath-on-substed-drive.js: drop (Windows-specific SUBST drive test, fails on Windows aarch64).
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 27, 2026

CLA assistant check
All committers have signed the CLA.

@nathanwhitbot nathanwhitbot force-pushed the fix/node-compat-iter36 branch from 52b0d57 to d7514ad Compare April 28, 2026 00:26
@nathanwhitbot
Copy link
Copy Markdown
Contributor Author

Both fails on run 25026961018 are infra:

  • lint debug windows-x86_64: JSR cache miss (Failed reading cache entry for 'https://jsr.io/@std/yaml/1.1.0_meta.json')
  • test node_compat (2/3) release linux-x86_64: sysroot decompress exited 8 before any test ran

cc @nathanwhit for a rerun once infra recovers.

Copy link
Copy Markdown
Contributor

@fibibot fibibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Body lists 9 tests; diff has 4 net additions (the other 5 — test-child-process-exec-any-shells-windows, test-fs-realpath-on-substed-drive, test-http-keepalive-override, test-http-pipeline-assertionerror-finish, test-https-byteswritten — already on main via intermediate PRs, same merge-churn pattern as the recent batches).

Net additions checked alphabetically:

  • test-http2-max-concurrent-streams.js
  • test-http2-server-settimeout-no-callback.js
  • test-http2-socket-close.js
  • test-tls-startcom-wosign-whitelist.js

Holding off on APPROVE — CI has two red jobs but neither is actually caused by this PR's changes:

  1. lint debug windows-x86_64 — fails with Error: .github/workflows/start_release.generated.yml is out of date. Run: .github/workflows/start_release.ts. That's a workflow-regen check that's stale because this branch hasn't picked up a recent main commit that touched start_release.ts. A merge from main and re-running the generator will fix it; nothing for this PR's actual diff to address.

  2. test node_compat (2/3) release linux-x86_64 — exit code 8 during sysroot setup (download/extract from deno_sysroot_build), well before any test ran. Pure infrastructure flake; a CI re-run should clear it.

Per fibibot's new CI gate I can't autonomously APPROVE while there are red rows, even when the failures are infrastructural. Quick fix is to rebase on main + click "re-run" on the linux job. Once those land green I'll re-confirm.

131/134 passing checks, all relevant test node_compat debug and test unit_node debug are clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants